Skip to content

refactor: route doctor through command module#164

Closed
ndycode wants to merge 2 commits intorefactor/pr1-fix-command-2from
refactor/pr1-route-doctor-direct
Closed

refactor: route doctor through command module#164
ndycode wants to merge 2 commits intorefactor/pr1-fix-command-2from
refactor/pr1-route-doctor-direct

Conversation

@ndycode
Copy link
Owner

@ndycode ndycode commented Mar 21, 2026

Summary

  • remove the remaining inline doctor wrapper from lib/codex-manager.ts
  • route codex auth doctor directly through the extracted doctor command module

What Changed

  • deleted the now-redundant inline runDoctor() wrapper
  • updated the doctor dispatch path to call runDoctorCommand(...) directly with the existing injected helpers and storage services

Validation

  • npm run test -- test/codex-manager-doctor-command.test.ts test/codex-manager-cli.test.ts
  • npm run lint
  • npm run typecheck
  • npm run build

Risk and Rollback

  • Risk level: low
  • Rollback plan: revert 231d9bb to restore the inline doctor wrapper

Additional Notes

  • this keeps the stacked manager split moving by removing another dispatcher-only wrapper without changing doctor behavior

note: greptile review for oc-chatgpt-multi-auth. cite files like lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.

Greptile Summary

removes the one-hop runDoctor wrapper in lib/codex-manager.ts and calls runDoctorCommand directly from the "doctor" dispatch branch. the dep object is byte-for-byte identical between the deleted wrapper and the new inline site; typescript's structural typing enforces completeness so no dep can silently go missing in the future.

  • the only file changed is lib/codex-manager.ts
  • no behavioral change — the call graph is shortened by one frame, nothing else moves
  • existing coverage in test/codex-manager-cli.test.ts (three doctor dispatch tests at lines ~5719-5829) plus test/codex-manager-doctor-command.test.ts already exercises the full code path
  • no windows filesystem paths, token storage, or concurrency surfaces are touched by this change

Confidence Score: 5/5

  • safe to merge — pure structural cleanup with no behavioral surface change
  • the wrapper was a transparent pass-through; the dep set is unchanged; typescript enforces completeness; three existing cli integration tests plus the dedicated doctor-command unit suite cover the dispatch path end-to-end
  • no files require special attention

Important Files Changed

Filename Overview
lib/codex-manager.ts removes the runDoctor pass-through wrapper and inlines the identical runDoctorCommand call directly in the dispatcher; dep set is unchanged and typescript enforces completeness

Sequence Diagram

sequenceDiagram
    participant CLI as runCodexMultiAuthCli
    participant OLD as runDoctor (removed)
    participant CMD as runDoctorCommand

    Note over CLI,CMD: before this PR
    CLI->>OLD: runDoctor(rest)
    OLD->>CMD: runDoctorCommand(rest, deps)

    Note over CLI,CMD: after this PR
    CLI->>CMD: runDoctorCommand(rest, deps)
Loading

Reviews (2): Last reviewed commit: "Merge remote-tracking branch 'origin/ref..." | Re-trigger Greptile

@chatgpt-codex-connector
Copy link

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 21, 2026

Warning

Rate limit exceeded

@ndycode has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 18 minutes and 38 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 385e391e-494a-4b98-86c0-5e3069d68549

📥 Commits

Reviewing files that changed from the base of the PR and between 4222e88 and ab68aa8.

📒 Files selected for processing (1)
  • lib/codex-manager.ts
📝 Walkthrough

Walkthrough

the fix cli command is extracted from lib/codex-manager.ts into a dedicated module lib/codex-manager/commands/fix.ts using dependency injection. the main entry point delegates to the new command handler rather than implementing logic inline. a test file covers basic invocation scenarios.

Changes

Cohort / File(s) Summary
Main Entry Point Refactoring
lib/codex-manager.ts
removed inline fix command implementation (types, control flow, helpers) totaling ~465 lines. now delegates runFix(args) to imported runFixCommand(args, deps) with injected utilities for storage, quota cache, token refresh, forecasting, and output. also refactored doctor command dispatch to call runDoctorCommand directly. removed unused TokenFailure import from types.
Fix Command Module
lib/codex-manager/commands/fix.ts
new 538-line module implementing complete fix workflow. exports FixCliOptions, FixAccountReport, FixCommandDeps interfaces and runFixCommand(), summarizeFixReports() functions. handles arg parsing, account iteration with per-account refresh/probe logic, hard-disable vs soft-warning categorization on refresh failures, quota cache updates, forecast evaluation, and json/tty output formatting.
Fix Command Tests
test/codex-manager-fix-command.test.ts
new 92-line vitest suite covering three scenarios: --help flag returns exit 0, invalid --bad option returns exit 1 with error logged, and empty storage returns exit 0 with info logged. uses stubbed deps via createDeps() helper to isolate external side effects.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Notes

missing regression tests: test/codex-manager-fix-command.test.ts:1-92 lacks coverage for core fix logic—account refresh, hard-disable with re-enable logic to prevent lockout, quota cache mutations in live mode, forecast evaluation, and json output structure. the three test cases only validate error paths and empty state.

concurrency risk: lib/codex-manager/commands/fix.ts mutates and saves quota cache in live mode. if multiple fix command instances run concurrently, quota cache edits could race without file-level locking.

windows edge case: token identity/email pruning logic in lib/codex-manager/commands/fix.ts (likely around quota-email cache cleanup) should be validated on windows paths, especially if using forward slashes vs backslashes in cache keys.

dependency injection surface: FixCommandDeps interface is substantial; ensure all callers at lib/codex-manager.ts:runFix() correctly populate all required deps fields—missing utilities could cause silent null-reference errors at runtime.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed Title follows conventional commits format with type 'refactor', scope omitted as acceptable, and summary clearly describes the main change of routing doctor through the command module.
Description check ✅ Passed Description covers summary, what changed, validation steps (with checkmarks), risk/rollback plan, and additional notes. All major sections from template are present and substantive.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/pr1-route-doctor-direct
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch refactor/pr1-route-doctor-direct

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ndycode ndycode changed the base branch from refactor/pr1-doctor-command to refactor/pr1-fix-command-2 March 21, 2026 03:01
@ndycode
Copy link
Owner Author

ndycode commented Mar 23, 2026

Closing because this work is now included in main via #318 and #319.

@ndycode ndycode closed this Mar 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant